Skip to content

fix: prevent orphaned crawl record when lock check fails in StartCrawler - #47

Merged
StJudeWasHere merged 4 commits into
StJudeWasHere:mainfrom
RobinAngele:fix/orphaned-crawl-record-on-lock-error
May 23, 2026
Merged

StJudeWasHere merged 4 commits into
StJudeWasHere:mainfrom
RobinAngele:fix/orphaned-crawl-record-on-lock-error

Conversation

@RobinAngele

@RobinAngele RobinAngele commented May 19, 2026 •

Copy link
Copy Markdown

Problem

When two crawl requests arrive in rapid succession (or a scheduler fires
slightly early), StartCrawler could leave a zombie crawl row in the
database with end = NULL, permanently blocking all future crawls with
"project is already being crawled" — even after the real crawl finished.

Root cause

// Before this fix — SaveCrawl is called BEFORE the in-memory lock check
previousCrawl := s.repository.GetLastCrawl(&p)  // ← wasted DB query on rejection
crawl, err := s.repository.SaveCrawl(p)          // ← inserts row, end = NULL
...
c, err := s.addCrawler(u, &p, &b)                // ← lock already held → error
if err != nil {
    return err   // ← orphaned row never gets end set
}

SaveCrawl writes the DB record before addCrawler checks the
in-memory crawlers map. When addCrawler returns an error, the function
exits early and the goroutine that would call UpdateCrawl (which sets
end) is never started — leaving the row stuck forever.

Fix

Acquire the in-memory lock first, then do all DB writes. GetLastCrawl is
also moved after the lock check since its result is only used inside the
goroutine — no point querying the DB on a request that will be rejected.

c, err := s.addCrawler(u, &p, &b)        // lock first — fail fast, zero DB access
if err != nil {
    return err
}

previousCrawl := s.repository.GetLastCrawl(&p)   // only runs if lock was acquired

crawl, err := s.repository.SaveCrawl(p)           // only write if lock succeeded
if err != nil {
    s.removeCrawler(&p)                            // release lock if DB write fails
    return err
}

Changes

  • internal/services/crawler.go — reorder addCrawler before GetLastCrawl
    and SaveCrawl; release the lock if SaveCrawl fails.
  • internal/services/crawler_test.go — new test
    TestStartCrawlerNoDuplicateDBRecord asserts that a second concurrent
    StartCrawler call returns an error and does not invoke SaveCrawl.

Test plan

  • go test ./internal/services/... passes including the new test.
  • Trigger two rapid crawl requests for the same project; the second
    should be rejected and no new row with NULL end appears in crawls.
  • Normal single crawl completes and the row gets its end timestamp set.
  • If SaveCrawl fails (e.g. DB down), no orphaned in-memory lock entry
    remains and the project can be crawled once the DB recovers.

🤖 Generated with Claude Code

Ataman100b and others added 4 commits May 19, 2026 05:53
SaveCrawl was called before addCrawler checked the in-memory lock, so a
concurrent or rapid trigger could create a DB crawl row with a NULL end
timestamp and then return an error — leaving that row permanently stuck
and blocking all future crawls with "project is already being crawled".

Move addCrawler before SaveCrawl so no DB record is written when the
lock is already held. If SaveCrawl then fails, removeCrawler cleans up
the lock entry so the caller can retry cleanly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Move GetLastCrawl to after addCrawler so a rejected duplicate trigger
skips the DB round-trip entirely — the result is only needed inside the
goroutine which never starts when the lock is already held.

Add TestStartCrawlerNoDuplicateDBRecord to assert that a second
StartCrawler call while a crawl is in progress returns an error and
does not invoke SaveCrawl, preventing the orphaned NULL-end-timestamp
that blocked all future crawls.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The type=sha,prefix={{branch}}- rule produces a tag like '-d6926f4'
on PR events because {{branch}} evaluates to empty — Docker rejects
tags that start with a dash. Restrict the sha tag to the default
branch only, where {{branch}} is always 'main'.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fork PRs cannot write to the upstream registry — the GITHUB_TOKEN is
read-only for packages in that context. Build the image on PRs to keep
Dockerfile validation, but only push on branch push events.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@StJudeWasHere
StJudeWasHere merged commit 880b312 into StJudeWasHere:main May 23, 2026
2 checks passed
@StJudeWasHere

Copy link
Copy Markdown
Owner

Good catch. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants